Skip to content

Heap::extend_with_region() implementation #41

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

Heap::extend_with_region() implementation #41

wants to merge 2 commits into from

Conversation

MarcoCicognani
Copy link
Contributor

Hi Phil,

  • Added Heap::extend_with_region(&mut self, start_addr: usize, size: usize)
  • Added Heap::contiguous field and Heap::is_contiguous() property
  • Modified Heap::bottom() -> usize & Heap::top() -> usize to Heap::bottom() -> Option<usize> & Heap::top() -> Option<usize> to return Some only when Heap::contiguous is true

@phil-opp
Copy link
Member

Thanks for the pull request and sorry for the delay. While I understand your use case in #40, I don't think that modifying the Heap type is the best solution for this. My problem with it is that the API gets more complex and that the top and bottom methods might return None now. I would prefer to implement your second suggestion instead:

Otherwise you could make public the HoleList module to allow external uses.

This way, we keep the general API simple while still supporting users with more complex use cases. Would this work for you?

In case we implement this, I think we should move the align_layout function into the HoleList, so that the Heap type becomes just a trivial wrapper around HoleList.

@MarcoCicognani
Copy link
Contributor Author

Hi Phil, don't worry for the late.
Yes, I think that your solution is the best way to cover my needs

@MarcoCicognani MarcoCicognani deleted the extend_with_region branch December 28, 2020 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@MarcoCicognani @phil-opp